-
Notifications
You must be signed in to change notification settings - Fork 4
Allow developer to add custom FeedGeneratorService implementations #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow developer to add custom FeedGeneratorService implementations #44
Conversation
Wow @BenWhite27! I appreciate the amount of effort you put into this plugin. Right now, we're focusing on the release of 16 and Codegarden, so this PR won't be reviewed over the next couple of weeks. I'm sure we'll take a look at it after those events |
No worries, hope Codegarden goes well! |
If you've finished then mark it as ready for review and I'll take a look later. |
…mplementation to obtain IProductFeedGeneratorService instances via this collection.
… to declare the format they generate. Use this format declaration to return the correct output within ProductFeedController.
…s a developer aid.
beedf25
to
3e86bb7
Compare
Hi Dinh, Hope Codegarden went well! |
Wonderful @BenWhite27 ! I've scheduled few days at the last week of July to review this one 💪 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR allows package consumers to implement their own feed generators by introducing a collection-based architecture for feed generation. The changes move away from a hardcoded enum-based approach to a flexible system where custom feed generators can be registered and used.
- Introduced
FeedGeneratorCollection
and associated factory to allow registration of custom feed generators - Replaced
ProductFeedType
enum with string-basedFeedGeneratorId
for identifying generators - Added support for both XML and JSON feed formats through the new
FeedFormat
enum and corresponding generation methods
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
File | Description |
---|---|
ProductFeedController.cs | Updated to handle both XML and JSON feed generation based on generator format |
ProductFeedSettingController.cs | Modified to use the new feed generator collection instead of hardcoded enum values |
IUmbracoCommerceBuilderExtensions.cs | Added FeedGenerators collection builder and registered GoogleMerchantCenterFeedService |
CustomRouting.cs | Updated route action name from "Xml" to "Generate" |
ProductFeedSettingsService.cs | Updated validation to use feed generator collection instead of enum parsing |
InfrastructureMappingProfile.cs | Added mapping between FeedGeneratorId and database FeedType column |
ProductFeedType.cs | Removed enum file as it's replaced by collection-based approach |
ProductFeedSettingWriteModelValidator.cs | Updated validation to use FeedGeneratorId instead of FeedType |
ProductFeedSettingWriteModel.cs | Replaced FeedType property with FeedGeneratorId string property |
ProductFeedSettingReadModel.cs | Replaced FeedType property with FeedGeneratorId string property |
ProductFeedGeneratorFactory.cs | Refactored to use collection-based lookup instead of switch statement |
GoogleMerchantCenterFeedService.cs | Updated to inherit from base class and implement new interface properties |
FeedGeneratorServiceBase.cs | Added abstract base class to aid custom feed generator development |
FeedGeneratorCollectionBuilder.cs | Added collection builder for registering feed generators |
FeedGeneratorCollection.cs | Added collection class for feed generators |
IProductFeedGeneratorService.cs | Updated interface with Id, DisplayName, Format properties and separate XML/JSON methods |
IProductFeedGeneratorFactory.cs | Updated factory interface to accept string instead of enum |
FeedFormat.cs | Added enum for feed format types (Unknown, Xml, Json) |
details.element.ts | Updated frontend to use feedGeneratorId instead of feedType |
types.ts | Updated TypeScript types to use feedGeneratorId |
context.ts | Removed hardcoded Google Merchant Center property mappings from default state |
...mmerce.ProductFeeds.Core/Features/FeedGenerators/Application/IProductFeedGeneratorService.cs
Outdated
Show resolved
Hide resolved
...mmerce.ProductFeeds.Core/Features/FeedGenerators/Application/IProductFeedGeneratorService.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Commerce.ProductFeeds.Infrastructure/Implementations/ProductFeedSettingsService.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Commerce.ProductFeeds.Infrastructure/Implementations/ProductFeedSettingsService.cs
Outdated
Show resolved
Hide resolved
....ProductFeeds.Core/Features/FeedGenerators/Implementations/FeedGeneratorCollectionBuilder.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Commerce.ProductFeeds.Core/Features/FeedGenerators/Application/FeedFormat.cs
Outdated
Show resolved
Hide resolved
...mmerce.ProductFeeds.Core/Features/FeedGenerators/Implementations/FeedGeneratorServiceBase.cs
Outdated
Show resolved
Hide resolved
Hi @BenWhite27 , overall the PR looks great. However, it creates a couple of breaking changes. I've fixed the Edit: Never mind, I'm going to add a task in the build pipeline to verify the breaking changes. |
Great, Thanks for looking at this Dinh, apologies for the breaking changes, I think I had the mind set of this being part of a major release at the time and I'm not that used to writing library code. Let me know if there's anything else I can do. |
…sion of the actions. Improve the backward compatibility.
Hi @BenWhite27 , I've double checked that there's no API breaking changes left by adding a bunch of obsolete properties and methods. But, I've readded the default feed property mapping for the google feed in the front-end code because it's convenient and we have no alternatives for now. Can you verify that your changes remains? |
Thanks @umbracotrd I'll check ASAP |
HI @umbracotrd Cheers |
@BenWhite27 : great. I'll prepare the 16.1.0 and release it tomorrow |
@umbracotrd Great news, thanks for putting the finishing touches on this one. |
This PR allows package consumers to implement their own feed generators.
Key changes
FeedGenerators
Collection Builder and associated factory to allow consumers to specify additional generators in startup configuration.Id
,DisplayName
, andFormat
properties toIProductFeedGeneratorService
in the same implementation style as Property ExtractorsProductFeedController
updated to generate the correct feed type.For consideration
FeedGeneratorId
property of FeedSettings to the original database columnFeedType
. This should mean that existing GoogleMerchantFeeds will continue to work without changes but makes things a bit inconsistant, so perhaps a Migration here would be better?